Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade peer dependency versions #101

Merged
merged 2 commits into from
Mar 1, 2019
Merged

Conversation

kadamwhite
Copy link
Contributor

@kadamwhite kadamwhite commented Oct 30, 2018

It's that time again: time to upgrade our peer dependencies! This does bump the eslint version, so if @joehoyle @rmccue I acknowledge we may want to exercise more caution and hold off for a bit. That said, I'm in favor of testing this and then releasing as 0.6.0 to permit projects to take advantage of the changes. Because of how semver handles <1.0 versions a 0.6.0 release won't affect any projects currently using ^0.5.0, making the change strictly opt-in and forward-facing.

The other benefit is that there was a conflict between eslint-config-react-app and eslint-plugin-jsx-a11y, which the latest bump to the React App config mercifully resolves.

Resolves #56

This finally avoids a warning from eslint-config-react-app
Copy link
Contributor

@ntwb ntwb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, remove the jsx-a11y/href-no-hash rule from the config.

"babel-eslint": "^9.0.0",
"chalk": "^2.4.1",
"eslint": "^5.8.0",
"eslint-config-react-app": "^3.0.5",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 3.x release of eslint-config-react-app/index.js no longer includes the jsx-a11y/href-no-hash rule, not sure what release it was removed from, but the following should be removed from HM's config here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it was removed from there, I'd like to separate the upgrade from the decision of whether we should be removing that rule.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will address this in a follow-up PR on top of this PR.

packages/eslint-config-humanmade/package.json Outdated Show resolved Hide resolved
packages/eslint-config-humanmade/package.json Outdated Show resolved Hide resolved
packages/eslint-config-humanmade/package.json Outdated Show resolved Hide resolved
@ntwb
Copy link
Contributor

ntwb commented Oct 31, 2018

Via https://eslint.org/docs/user-guide/migrating-to-5.0.0


https://eslint.org/docs/user-guide/migrating-to-5.0.0#eslint-recommended-changes

  • Two new rules have been added to the eslint:recommended config:

    • for-direction enforces that a for loop update clause moves the counter in the right direction.
    • getter-return enforces that a return statement is present in property getters.

Worth noting in the release docs that these new rules are enabled by default.


https://eslint.org/docs/user-guide/migrating-to-5.0.0#rule-default-changes

  • The default options for the object-curly-newline rule have changed from { multiline: true } to { consistent: true }.

The 'consistent': true, values in the HM config could be updated to use this new default

@pdewouters pdewouters requested review from pdewouters and removed request for pdewouters November 2, 2018 09:14
@mikeselander mikeselander mentioned this pull request Feb 15, 2019
Copy link
Member

@joehoyle joehoyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Providing this is well tested, I have no issues with merging this.

@mikeselander mikeselander merged commit 1ca09ea into master Mar 1, 2019
@mikeselander mikeselander deleted the update-peer-dependencies branch December 27, 2019 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update eslint-config-humanmade peer dependencies
6 participants